U4 — per-task partial stream state + cleanup primitives

Split unit U4 of the upstream PR 1066 split, under the plan on the tracking issue (1989). Base is main per the merge order.

Related issue: 1934 (unit issue; U4 does not close the epic — the unit chain does).

Scope: per-task partial-stream state and the cleanup primitives around the streamed write. What this unit actually does at this head:

1. keys the partial-stream bookkeeping per task (taskId.instanceId) instead of the BaseTool singleton: each streaming write_to_file registers one entry plus one TaskAborted teardown listener, released by exactly one owner (clearTaskState(), releasePartialStreamBookkeeping());
2. stabilizes the streamed path per task before any UI or filesystem touch, and re-checks stream liveness (identity, not presence) after every await in handlePartial() so a cancelled task gets no re-ask, no re-opened diff view, and no streamed resurrection;
3. gives every execute() exit — three validation returns, both approval denials, success, and the catch — one shared release path, and gives the presenter's rejection paths (validation throw, repetition guard, parse failure) their own release boundary (releaseStreamAfterValidationRejection(), releaseStreamStateOnParseFailure());
4. owns the unapproved-preview teardown: the discard (memory-only, never a save) runs before reset() on every denial/failure exit, and the directories a delta adopted before open() are released on the no-editor exits;
5. wires the ClineProvider disposal: cleanupFailedHistoryTask() releases the tool's per-task state before task.dispose() (dispose removes the listeners the release needs) and only behind the PendingActionSettlementError + registry identity guard;
6. keeps saveChanges() ownership honest: the placeholder stays owned until the approved write lands, and a save that reports failure now throws a typed DocumentSaveFailedError before the placeholder release and any success bookkeeping, so the caller's failure handling removes what this edit created;
7. guarantees the execute() catch teardown: the discard, reset, and per-task release run in a finally around the presenter's handleError() report, so a report that rejects (an aborted task breaks Task.say()) can no longer leak the entry, its listener, and the preview — on both the diff-view branch and the prevent-focus-disruption saveDirectly branch.

Accepted divergence: sibling streaming tools (ApplyDiffTool, EditFileTool, SearchReplaceTool, EditTool) still use BaseTool's singleton lastSeenPartialPath/resetPartialState; lifting the per-task keying to BaseTool is a follow-up PR. The focused cleanup spec is sanctioned new content (allowNew): the primitives' catch arms are only reachable by calling them directly at this layer.

Content source of record: 72143527fd33306e5541116093c2cbf803cce9e0..pr-1066-audit (local), plus the chain ports and row fixes listed under "Updated this round"; zdt split verify PASSed at the tagged unit head, and every later port is pinned by its own negative control instead of a re-tag.

Budget (own delta, not the stacked view): 11 files, +4068 / −96 against the merge-base with main (707f1442d). Above the 1000 a+d soft cap — documented deviation: the unit owns the per-task state machine and its catch arms, which are only reachable through the tool-level specs; the split rationale is on the tracking issue (1989).

How to test:

pnpm --dir src exec vitest run --globals integrations/editor/__tests__/DiffViewProvider.spec.ts                    # 101 passed
pnpm --dir src exec vitest run --globals core/tools/__tests__/writeToFileTool.spec.ts                              # 62 passed / 5 skipped
pnpm --dir src exec vitest run --globals core/tools/__tests__/writeToFileTool-partial-state-cleanup.spec.ts        # 9 passed
pnpm --dir src exec vitest run --globals __tests__/removeClineFromStack-delegation.spec.ts                         # 26 passed
pnpm --dir src exec vitest run --globals core/assistant-message/__tests__/presentAssistantMessage-validation-rejection.spec.ts  # 6 passed
pnpm --dir src exec tsc --noEmit                                                                                   # clean on CI
pnpm --dir src exec eslint --max-warnings=0 src/integrations/editor/DiffViewProvider.ts src/core/tools/WriteToFileTool.ts src/core/webview/ClineProvider.ts src/core/assistant-message/presentAssistantMessage.ts src/core/tools/BaseTool.ts

Expected: all green, and src/eslint-suppressions.json unchanged.

Verification at this head (4645d26ab): DiffViewProvider spec 105 passed; writeToFileTool spec 72 passed / 5 skipped; partial-state-cleanup spec 9 passed; removeClineFromStack-delegation spec 26 passed; validation-rejection spec 8 passed. tsc --noEmit: no errors in any file this unit touches (the local donor chain carries 62 pre-existing @roo-code/types dist artifacts, identical before and after this commit; CI compile is green). ESLint --max-warnings=0: 0 errors / 0 warnings on all five touched files. Prettier: clean on the PR merge commit (848de9150, CRLF-normalized tree, --check . exit 0). src/eslint-suppressions.json untouched. Negative controls for this round's fixes: reverting the save-boolean block reddens exactly the new save-false test (1 failed / 101); reverting the try/finally reddens exactly the two report-reject tests (2 failed / 67); disabling the registry identity guard reddens exactly the two stale-task tests (2 failed / 26); removing the landed-write flag guard reddens exactly the discard-skip test (1 failed / 63); reverting the different-tool repetition negative to the pre-tier shape reddens exactly that test (1 failed / 8), as does forcing the hard-block name guard to always release (1 failed / 8); making the teardown ignore the commit state reddens exactly the two landed-write tests (2 failed / 66); skipping the release in the missing-parameter or rooignore finally reddens exactly its test plus the matching early-exit release test (2 failed / 66 each); removing the provider commit line reddens exactly the two new provider tests (2 failed / 104); making the shared early-exit cleanup skip the discard reddens exactly the five tests that assert a discard through that path (5 failed / 69); dropping the post-update() liveness check reddens exactly the new in-flight cancellation test (1 failed / 69); dropping the cancellation-teardown discard report reddens exactly its test (1 failed / 70); removing the pre-prompt hard-block release reddens exactly the two hard-block release tests (2 failed / 9); dropping the parse-failure directory release reddens exactly its test (1 failed / 71); reducing the map key to taskId alone reddens exactly the composite-key test and the existing isolation test (2 failed / 72); turning the pre-prompt release into a fire-and-forget call reddens exactly the completion-order test (1 failed / 9); clearing placeholder ownership only after a successful unlink reddens exactly the failed-delete test (1 failed / 105) — each restored byte-exact with hashes verified. No .changeset file, no CHANGELOG edit.

Chain position. Declared merge order: U3 (1931) -> U4 (this PR) -> U5 (1930) -> U6 (1932); U7 (1928) closes the chain as the final integration PR. Unit branches are not cumulative: fixes to shared code are ported to each branch that carries it, re-derived per branch.

Updated this round (head 237ee4180 -> 4645d26ab -> 4a1894e3e -> 25ac09b89 -> 03166ac06 -> 02e77b95b -> affdc7033 -> 9d2ab7517 -> c4ec46909 -> a30bedbde -> bef984c4b -> 8a2465d67 -> 0897b245f)

Content commit 0897b245f clears the Persistence Integrity row from the walkthrough at bef984c4b: a denied new-file write calls revertChanges(), whose fs.unlink() can fail (EBUSY on Windows is the everyday case), and the old code cleared placeholderPath only after a successful unlink - so the failed revert left the ownership claim alive, the execute catch ran the discard, and the discard retried the unlink against a path anything may have recreated in the meantime (the user's new file, the next edit's placeholder). Ownership is now terminal at the ATTEMPT - the same semantics the discard uses when it clears the field before unlinking: a failed delete surfaces as the revert's rejection (the artifact may remain and is reported) but never as a claim a later cleanup retries against a stranger's file. Regression test: revert whose unlink rejects, then a discard - the path is unlinked exactly once (negative control: clear-after-success reddens exactly that test, 1 failed / 105).

Content commit 8a2465d67 clears the one open thread recorded at bef984c4b (replied and resolved): the completion-order gap in the pre-prompt release test - the release mock resolved immediately, so the invocation-order assertion proved only call order and survived a fire-and-forget mutant. The test now holds the release promise pending, waits until the release has been called, asserts the prompt has not opened while cleanup is in flight, resolves the release, and only then lets the prompt run and reject (negative control: a fire-and-forget release reddens exactly this test, 1 failed / 9).

Content commit bef984c4b clears the Regression Evidence row from the walkthrough at a30bedbde: the composite taskId.instanceId keying had no focused coverage - every existing isolation test uses different taskIds, so a taskId-only key passed them all. A new test registers state for two task doubles that share a taskId and differ in instanceId, asserts two independent entries and distinct abort listeners, and asserts that releasing the replaced instance leaves the live instance's entry and listener untouched (negative control: a taskId-only key reddens exactly this test and the existing isolation test, 2 failed / 72). The Lifecycle Resource Cleanup row re-raised at a30bedbde is the same duplicate-teardown claim argued above; the registered argument now addresses its sharper save-race case explicitly.

Content commit a30bedbde clears the Persistence Integrity row from the walkthrough at 9d2ab7517: a new-file delta adopts the parent directories before the diff view exists, an empty-content stream skips open() so no editor ever owns them, and the parse-failure teardown reset the provider - reset() drops the adopted list without touching disk - leaving the directories of a write that never happened on disk with no owner. The teardown now calls releaseEarlyDirectories() between the discard and the reset, the same boundary the validation-rejection release and the early exits use; a regression test streams empty content, fails the completed block's parse, and asserts the adopted directories are removed (negative control: dropping the call reddens exactly that test, 1 failed / 71).

Registered argument - Lifecycle Resource Cleanup (re-raised at a30bedbde, duplicate teardown after cancellation): the overlap is real, predates this unit, and its worst case is bounded. Task.disposeOnce() starting revertChanges() while an in-flight update() still writes into the document is a window Task already has - the new liveness check does not create the overlap, it adds a guarded discard after it. The sharper claim, that revertChanges() can save the unapproved streamed buffer before the discard empties it, is bounded by the discard's own contract: for a new file the discard's final step unlinks the file, so any stale intermediate save is removed with it; for a modify both paths end by restoring the original content, and removeAdoptedDirectories() takes its list before its first await and tolerates ENOENT/ENOTEMPTY on a second pass. Without the fallback the leak is guaranteed whenever the dispose path's isEditing check ran before this delta's streamed lines landed; with it, the worst case is a second no-op pass or a stale save that the discard then removes. The proposed shared cleanup contract across Task.dispose() and the tool's teardown is a redesign of the task-disposal path whose blast radius exceeds this unit; this unit's coordination stays the isEditing guard plus idempotent primitives.

Content commit c4ec46909 clears the three open threads recorded at 9d2ab7517 (all replied and resolved): teardown discard failures were dropped - on the cancellation path the chat cannot receive the report (Task.abortTask() sets abort before it emits TaskAborted, and Task.say() rejects once abort is set), so teardownCancelledPreview() now routes the returned error through the provider's output channel, the user-visible boundary that survives the abort; the hard-block branch released the refused write's stream AFTER awaiting the guidance prompt - the state, listener, and unapproved preview were held hostage to an answer that may never come, and a rejected ask() skipped the release entirely; the release now runs before the prompt opens, and a new test rejects the prompt and asserts the release precedes the ask. The two read_file negative controls asserted only mock call counts; they now assert the tool_result the model received (content and tool_use_id).

Main moved again under a green branch (CI runs the merge commit): 707f1442d froze the tool policy into a required request-snapshot parameter on presentAssistantMessage(), and this unit's rejection spec called the presenter with the task only - all eight of its tests died on the new guard in the ubuntu run. Merge commit 3ba1efb74 takes main in (the unit's presenter edits sit in the repetition branch and merged cleanly beside the new guard); commit 9d2ab7517 passes the same inert baseSnapshot the other presenter specs use. The worktree holds no single-argument call sites left; the sibling presenter specs main updated pass alongside (auto-deny 13, unknown-tool 17, images 7).

Content commit affdc7033 clears the two real rows from the review at 02e77b95b: Persistence Integrity (error) — streaming can open the diff view and fill it with unapproved content before the completed call reaches execute(); the missing-parameter exits only released bookkeeping and reset, and reset() alone leaves the diff document dirty while closeAllDiffViews() skips dirty diff tabs, so a malformed completed call left the streamed content in a live tab for a later user save to persist. The three early exits (missing path, missing content, rooignore denial) now share one cleanup path — release this task's state, discard a preview streaming had opened (a discard failure is reported explicitly), drop directories no editor took over, reset — in the finally around the exit's report, so the missing-parameter result stays the primary result. Lifecycle Resource Cleanup (warning) — handlePartial() checked stream liveness before open() and before update(), but not after update() completed; a cancellation landing while VS Code's edit application awaits left the delta updating decorations for a cancelled task and returning with the preview active on the disposal path. The delta that streamed now runs the same ownership-safe teardown as the post-open() cancellation branch (extracted as teardownCancelledPreview()) and returns. Three regression tests: preview discard on each missing-parameter exit (with discard-before-reset ordering and the missing-parameter result preserved), and cancellation released while update() is in flight.

Content commit 03166ac06 clears the two real rows from the review at 25ac09b89: Persistence Integrity (error) — the landed-write flag was one await late: saveChanges() records the commit when it RETURNS, but it keeps awaiting after the save lands (tab handling, preview restoration, diagnostics), and a rejection there sent the teardown into a discard of a write already on disk (for a create, the discard saves an empty buffer over the approved content). DiffViewProvider now records the commit itself — approvedSaveCommitted is set the moment the content is on disk in both write paths and cleared by reset() — and the tool's teardown reads that state. Lifecycle Resource Cleanup (warning) — the two missing-parameter exits and the rooignore exit awaited their report before releasing; a rejected report escaped with the per-task map entry held (Task.dispose() drops listeners but never the entry). All three exits now run release, preview discard, directory cleanup, and guarded reset in a finally around the report; the rejection still propagates after the teardown. Formatting-only commit 02e77b95b: the prettier gate on the merge commit flagged one long line in the new provider test; prettier wrapped it, no token changed, the spec still passes 104.

Content commit 25ac09b89 clears the two inline threads from the review at 4a1894e3e: the catch teardown no longer touches an approved write that already landed — execute() records when saveDirectly() or saveChanges() resolves and the unapproved-preview discard is skipped from that point on (a post-save rejection — trackFileContext(), or pushToolWriteResult() reporting user edits on an aborted task — previously rmdir'd the directory holding the approved file with ENOTEMPTY, closed its tab, and could blank or revert a dirty buffer over approved content; the reset, the per-task release, and the report of the step that actually failed still run, and a mid-write saveChanges() failure keeps its discard); and the different-tool repetition negative reached the guard branches — it moved to the detector's real action: "hard_block" shape, asserts the refusal in the tool result, and gained a soft-block sibling, killing the surviving mutants on the branch's tool-name guard. Both threads replied to and resolved.

The review's remaining pre-merge warning (duplicate cleanup against an active task disposal, at the open() cancellation window) is argued, not coded: the coordination it asks for is already the mechanism. The delta's teardown at that window is guarded by isEditing — if the task's disposal already reverted the view, isEditing is false and the delta starts nothing; the window where the delta must act is the one documented at that site (the abort cleanup checked isEditing before open()'s session existed and missed the view, leaving it with no other owner). The primitives are idempotent under the remaining interleavings: the discard is a no-op with no view open, reset() is guarded and its failure contained, and releaseEarlyDirectories() both skips while a view is open and takes the directory list (createdDirs is emptied before the first await) so a second caller removes nothing and tolerates ENOENT/ENOTEMPTY regardless. No second cleanup can run against a disposal that owns the revert.

Merge commit 4a1894e3e brings in main 995167eed (required-check demand: the CI run at 4645d26ab executed on the PR merge commit against that main, whose tiered tool-repetition detection rewrote the presenter's repetition guard; the unit's release survived on the hard-block branch, and the guard's new soft-block exit — which also breaks before handle() — gained the same release, pinned by a new test; the unit's repetition test moved to the new action: "hard_block" check-result shape). Prettier clean on the resulting merge commit; negative control: removing the soft-block release reddens exactly the new test (1 failed / 7).

Content commit 4645d26ab clears the three at-head pre-merge rows:

• Persistence Integrity (error) — saveChanges() captured and dropped the document-save boolean, so a save reporting false still released the placeholder and reported success over a file that never changed. Now throws DocumentSaveFailedError before the placeholder release; the regression double resolves an explicit false. Shape ported from the sibling unit that fixed this first, re-derived for this branch's placeholder-release commit point; the existing landing test now mocks the real true contract.
• Lifecycle Resource Cleanup (warning) — the execute() catch teardown (discard, reset, per-task release) now runs in a finally around the presenter's handleError() report; a rejected report still propagates after the cleanup, so the failure surfaces exactly once. Two kill tests cover both branches (diff-view and prevent-focus-disruption saveDirectly). The row's handlePartial() clause needs no code change at this head: that boundary completes every cleanup (entry release or failed-stream mark, guarded discard, guarded reset) before it rethrows, so the presenter-side report in BaseTool's handle() cannot skip any of it, and the failed-stream suppression is deliberately retained until the teardown that ends the call — see the walkthrough's own description of the two-window ownership.
• Regression Evidence (warning) — two focused tests pin the cleanupFailedHistoryTask() identity guard: a PendingActionSettlementError for a task the registry dropped, and for one it replaced under the same id, leave the stale task's stream entry, its listeners, and its disposal untouched (the replacement keeps its registry slot).

Earlier rounds (kept for the record): the early-exit/liveness port (1d4a2a6a4/2f356e6f7), the main refresh onto 036245c5e (U1 content left the diff), the seven-exit shared release, and the fb21709cd row fixes (rollback-error reporting, pre-streaming boundary, four focused tests).

Pre-Submission Checklist

• Issue Linked: This PR is linked to an approved GitHub Issue (1934, the unit issue of the approved split recorded on 1989).
• Scope: The changes are focused on the linked issue: per-task partial-stream state and the cleanup primitives around the streamed write.
• Self-Review: The code was reviewed with care; every teardown branch names its owner.
• Writing Style: The PR title and description use short sentences, plain words, and active voice.
• Testing: New and updated tests cover the changes at the lowest valid layer; each fix names the test that reddens for its negative control.
• No breaking changes: No public API, setting, or webview message changed.

Documentation Updates

No user-facing behavior, setting, or webview surface changed, so no documentation update is needed. The teardown ownership rules are documented in the code comments at each boundary.

Recreate policy

If the bot stalls on a pre-merge check and the existing head cannot obtain bot review/approval (empty-commit re-trigger attempted and failed), the unit is recreated from the tagged content source of record — never from a per-PR head. At most 1 PR per issue.

Linked issue

Closes #1934 (unit U4 of the upstream PR 1066 split).

Split-unit issue reference

The linked issue 1934 is the approved issue for this change. It is the unit issue filed under the declared split of a larger change: the split plan, the unit boundaries, the approval of the split, and this unit's acceptance criteria are recorded on the tracking issue in this repository: 1989. That tracking issue is also the home for this unit's review dispositions and follow-up registrations. "No approved upstream issue" would only mean the original single-PR change, which was split before any of these PRs opened.

Stryker directive sweep: removed the // Stryker disable next-line ConditionalExpression comment above the path-stabilization check in WriteToFileTool.ts per maintainer directive (mutation-diff is advisory; suppression comments are no longer accepted). Equivalence rationale preserved here: the !== undefined clause is redundant - when lastSeenPartialPath is undefined the second clause only matches an undefined partialPath, which the !!partialPath in the return value rejects either way, so no test can distinguish the two. Lint follow-up: upstream main re-enabled @typescript-eslint/no-unused-vars for src, which exposed dead bindings in this branch's test files on the merge ref. Cleared: the unused stateFor helper in writeToFileTool-partial-state-cleanup.spec.ts and the unused fsPath in the DiffViewProvider placeholder test. Focused suites re-run green; ESLint --max-warnings=0, tsc --noEmit and Prettier clean.

Verification at this commit: writeToFileTool suites 81 passed / 5 skipped; tsc --noEmit clean; ESLint --max-warnings=0 clean; Prettier clean (comment-only diff).